Skip to content

Fix Docker building (and also build for forks)#15389

Merged
koppor merged 2 commits intomainfrom
fix-docker-build
Mar 23, 2026
Merged

Fix Docker building (and also build for forks)#15389
koppor merged 2 commits intomainfrom
fix-docker-build

Conversation

@koppor
Copy link
Copy Markdown
Member

@koppor koppor commented Mar 23, 2026

Related issues and pull requests

Extracted from #15357

PR Description

Docker build was not working. This fixes it.

Steps to test

See CI passing OR run docker build . -t jabkit -f Dockerfile.jabkit

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [.] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix Docker building and enable builds for repository forks

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Remove repository restriction to enable Docker builds for forks
• Update Maven and JDK versions in Dockerfiles
• Add explicit module-info.java file copies to Docker build context
• Separate Docker build commands into individual RUN layers
Diagram
flowchart LR
  A["Docker Build Workflow"] -->|Remove repo check| B["Enable Fork Builds"]
  C["Dockerfile.jabkit/jabsrv"] -->|Update versions| D["Maven 3.9.14, JDK 26"]
  C -->|Add module-info copies| E["Include Java modules"]
  C -->|Separate RUN commands| F["Optimize layer caching"]
  B --> G["Fixed Docker Build"]
  D --> G
  E --> G
  F --> G
Loading

Grey Divider

File Changes

1. .github/workflows/dockerimages.yml ✨ Enhancement +0/-1

Enable Docker builds for repository forks

• Removed if: github.repository == 'JabRef/jabref' condition from build job
• This allows Docker image builds to run on forked repositories

.github/workflows/dockerimages.yml


2. Dockerfile.jabkit 🐞 Bug fix +16/-5

Update versions and fix module-info copying

• Updated Maven base image from 3.9.12 to 3.9.14
• Updated commented JDK alternative from 25 to 26
• Added explicit COPY commands for all module-info.java files from each component
• Separated build, dist directory creation, and artifact movement into individual RUN layers

Dockerfile.jabkit


3. Dockerfile.jabsrv 🐞 Bug fix +16/-5

Update versions and fix module-info copying

• Updated Maven base image from 3.9.12 to 3.9.14
• Updated commented JDK alternative from 25 to 26
• Added explicit COPY commands for all module-info.java files from each component
• Separated build, dist directory creation, and artifact movement into individual RUN layers

Dockerfile.jabsrv


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Mar 23, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. amazoncorretto commented FROM line 📘 Rule violation ⚙ Maintainability
Description
A commented-out FROM ... AS build line is kept in a modified section of the Dockerfile, which is
considered commented-out code. This increases maintenance noise and violates the requirement to
remove commented-out code in touched areas.
Code

Dockerfile.jabkit[4]

+# FROM amazoncorretto:26-jdk AS build
Evidence
PR Compliance ID 10 requires removing commented-out code in modified areas; the PR changes and
retains a commented-out FROM instruction in Dockerfile.jabkit.

AGENTS.md
Dockerfile.jabkit[4-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A commented-out Docker instruction (`# FROM ...`) remains in a modified part of the Dockerfile.
## Issue Context
The project compliance rules require removing commented-out code in touched sections.
## Fix Focus Areas
- Dockerfile.jabkit[4-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Fork images use upstream names🐞 Bug ✓ Correctness
Description
After removing the job-level repository guard, the workflow runs in forks but still generates tags
for ghcr.io/JabRef/... and labels pointing to https://github.com/JabRef/jabref, so fork builds
produce incorrect image metadata and cannot publish correctly-namespaced images on fork push/tag
events.
Code

.github/workflows/dockerimages.yml[25]

-    if: github.repository == 'JabRef/jabref'
Evidence
The PR removes the only job-level condition that prevented this workflow from running outside the
upstream repo. In the resulting workflow, docker/metadata-action still hard-codes the GHCR
namespace to JabRef and hard-codes the source label to the upstream repo URL, so any run in a fork
will produce upstream-branded tags/labels rather than fork-specific ones.

.github/workflows/dockerimages.yml[3-14]
.github/workflows/dockerimages.yml[23-29]
.github/workflows/dockerimages.yml[31-45]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Docker Images workflow now runs in forks (job-level guard removed), but still hard-codes the upstream GHCR namespace (`ghcr.io/JabRef/...`) and upstream `org.opencontainers.image.source` label. This causes fork builds to generate incorrect tags/labels and prevents forks from producing correctly-namespaced images on fork `push`/`tag` events.
### Issue Context
The workflow already gates login/push to upstream-only, but the metadata generation is still upstream-specific.
### Fix Focus Areas
- .github/workflows/dockerimages.yml[31-45]
- Change `images:` to use the current repo owner dynamically (e.g., `ghcr.io/${{ github.repository_owner }}/${{ matrix.component }}`).
- Change `org.opencontainers.image.source` to use the current repo (e.g., `https://github.com/${{ github.repository }}`).
- .github/workflows/dockerimages.yml[46-88]
- Revisit login/push gating if the intent is to allow forks to publish on their own `push`/`tag` events (otherwise keep upstream-only, but ensure tags/labels are still fork-correct for non-push builds).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Duplicated module-info COPY block 📘 Rule violation ⚙ Maintainability
Description
The PR introduces an identical block of COPY .../module-info.java lines in multiple Dockerfiles,
duplicating build logic. This increases maintenance overhead and risk of divergence when module
paths change.
Code

Dockerfile.jabkit[R35-42]

+COPY ./jabgui/src/main/java/module-info.java ./jabgui/src/main/java/module-info.java
+COPY ./jabkit/src/main/java/module-info.java ./jabkit/src/main/java/module-info.java
+COPY ./jablib/src/main/java/module-info.java ./jablib/src/main/java/module-info.java
+COPY ./jabls/src/main/java/module-info.java ./jabls/src/main/java/module-info.java
+COPY ./jabls-cli/src/main/java/module-info.java ./jabls-cli/src/main/java/module-info.java
+COPY ./jabsrv/src/main/java/module-info.java ./jabsrv/src/main/java/module-info.java
+COPY ./jabsrv-cli/src/main/java/module-info.java ./jabsrv-cli/src/main/java/module-info.java
+COPY ./test-support/src/main/java/module-info.java ./test-support/src/main/java/module-info.java
Evidence
PR Compliance ID 32 requires refactoring duplicated logic into shared helpers/workflow steps; the
same module-info.java COPY block is added in both Dockerfile.jabkit and Dockerfile.jabsrv.

Dockerfile.jabkit[35-42]
Dockerfile.jabsrv[35-42]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds the same `COPY .../module-info.java` list to multiple Dockerfiles, duplicating logic.
## Issue Context
Keeping duplicated Dockerfile logic in sync is error-prone when modules are added/renamed. Consider consolidating the shared build pre-copy logic into a single reusable approach (e.g., a shared base build stage/image, a single parameterized Dockerfile with `--build-arg`/`--target`, or another consolidation pattern acceptable for the repo).
## Fix Focus Areas
- Dockerfile.jabkit[35-42]
- Dockerfile.jabsrv[35-42]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo


# Is not debian-based
# FROM amazoncorretto:25-jdk AS build
# FROM amazoncorretto:26-jdk AS build
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Amazoncorretto commented from line 📘 Rule violation ⚙ Maintainability

A commented-out FROM ... AS build line is kept in a modified section of the Dockerfile, which is
considered commented-out code. This increases maintenance noise and violates the requirement to
remove commented-out code in touched areas.
Agent Prompt
## Issue description
A commented-out Docker instruction (`# FROM ...`) remains in a modified part of the Dockerfile.

## Issue Context
The project compliance rules require removing commented-out code in touched sections.

## Fix Focus Areas
- Dockerfile.jabkit[4-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@testlens-app

This comment has been minimized.

@testlens-app
Copy link
Copy Markdown

testlens-app bot commented Mar 23, 2026

✅ All tests passed ✅

🏷️ Commit: 090a26f
▶️ Tests: 10203 executed
⚪️ Checks: 68/68 completed


Learn more about TestLens at testlens.app.

@koppor koppor added the automerge PR is tagged with that label will be merged if workflows are green label Mar 23, 2026
@koppor koppor enabled auto-merge March 23, 2026 10:02
@koppor koppor added this pull request to the merge queue Mar 23, 2026
@github-actions github-actions bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Mar 23, 2026
Merged via the queue into main with commit 88c984d Mar 23, 2026
69 checks passed
@koppor koppor deleted the fix-docker-build branch March 23, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge PR is tagged with that label will be merged if workflows are green status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants